✅ Autofix completed

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

crates/grpc_client/src/mlx_engine.rs (1)

125-131: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Implement case-insensitive scheme matching.

The scheme checks use case-sensitive strip_prefix and won't handle mixed-case schemes like GRPC:// or GrPcS://. Past review analysis shows worker.rs:952 passes metadata.spec.url directly to GrpcClient::connect(), and since grpc_base_url in util.rs preserves the original URL casing when a scheme is detected, uppercase schemes will bypass the conversion and cause connection failures.

🔧 Proposed fix

-        let http_endpoint = if let Some(addr) = endpoint.strip_prefix("grpc://") {
+        let lower = endpoint.to_ascii_lowercase();
+        let http_endpoint = if let Some(_) = lower.strip_prefix("grpc://") {
+            let addr = &endpoint[7..];
             format!("http://{addr}")
-        } else if let Some(addr) = endpoint.strip_prefix("grpcs://") {
+        } else if let Some(_) = lower.strip_prefix("grpcs://") {
+            let addr = &endpoint[8..];
             format!("https://{addr}")
         } else {
             endpoint.to_string()
         };

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/grpc_client/src/mlx_engine.rs` around lines 125 - 131, The
scheme-matching for building http_endpoint in the mlx_engine.rs block (the
if/else that currently uses endpoint.strip_prefix("grpc://") and
strip_prefix("grpcs://")) is case-sensitive and fails for mixed/upper-case
schemes; update this block to perform case-insensitive detection (e.g. compare a
lowercased view of endpoint or use a case-insensitive starts-with) and then
strip only the scheme portion while preserving the original remainder of
endpoint so GrpcClient::connect() (and grpc_base_url in util.rs) receive the
correct host/path; adjust the logic around the http_endpoint variable and the
endpoint handling to use these case-insensitive checks.

crates/grpc_client/src/vllm_engine.rs (1)

145-152: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Implement case-insensitive scheme matching.

The scheme checks use case-sensitive strip_prefix and won't handle mixed-case schemes like GRPC:// or GrPcS://. Past review analysis shows that direct callers bypass workflow layer normalization, and since grpc_base_url in util.rs preserves the original URL casing when a scheme is detected, mixed-case schemes will fail the conversion and cause connection failures.

🔧 Proposed fix

         // Convert gRPC schemes to tonic-compatible HTTP(S) endpoints.
-        let http_endpoint = if let Some(addr) = endpoint.strip_prefix("grpc://") {
+        let lower = endpoint.to_ascii_lowercase();
+        let http_endpoint = if let Some(_) = lower.strip_prefix("grpc://") {
+            let addr = &endpoint[7..];
             format!("http://{addr}")
-        } else if let Some(addr) = endpoint.strip_prefix("grpcs://") {
+        } else if let Some(_) = lower.strip_prefix("grpcs://") {
+            let addr = &endpoint[8..];
             format!("https://{addr}")
         } else {
             endpoint.to_string()
         };

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/grpc_client/src/vllm_engine.rs` around lines 145 - 152, The current
scheme matching on `endpoint` uses case-sensitive `strip_prefix` and fails for
mixed-case schemes; update the conversion that sets `http_endpoint` so it
matches schemes case-insensitively by comparing a lowercased view (e.g., `let
lower = endpoint.to_lowercase()`) and, when a match is found, take the remainder
from the original `endpoint` by slicing with the scheme length (so casing of the
address is preserved) and then format to "http://{addr}" or "https://{addr}".
Make this change in the block that computes `http_endpoint` (the code
referencing `endpoint` and producing `http_endpoint`) so both "grpc://" and
"grpcs://" are handled regardless of case.

crates/grpc_client/src/sglang_scheduler.rs (1)

145-152: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Implement case-insensitive scheme matching.

The scheme checks use case-sensitive strip_prefix and won't handle mixed-case schemes like GRPC:// or GrPcS://. Past review analysis confirms that worker.rs:952 and other callers pass URLs directly without normalization, and mixed-case schemes will bypass the conversion logic causing connection failures.

🔧 Proposed fix

         // Convert gRPC schemes to tonic-compatible HTTP(S) endpoints.
-        let http_endpoint = if let Some(addr) = endpoint.strip_prefix("grpc://") {
+        let lower = endpoint.to_ascii_lowercase();
+        let http_endpoint = if let Some(_) = lower.strip_prefix("grpc://") {
+            let addr = &endpoint[7..];
             format!("http://{addr}")
-        } else if let Some(addr) = endpoint.strip_prefix("grpcs://") {
+        } else if let Some(_) = lower.strip_prefix("grpcs://") {
+            let addr = &endpoint[8..];
             format!("https://{addr}")
         } else {
             endpoint.to_string()
         };

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/grpc_client/src/sglang_scheduler.rs` around lines 145 - 152, The
scheme checks on endpoint use case-sensitive strip_prefix and miss mixed-case
schemes; update the conversion that sets http_endpoint to do case-insensitive
matching: create a lowercase copy (e.g., let lower =
endpoint.to_ascii_lowercase()), test lower.strip_prefix("grpc://") and
lower.strip_prefix("grpcs://"), and when a match is found compute the original
address suffix by slicing the original endpoint past the matched scheme length
(so you preserve original casing/host) and then format!("http://{addr}") or
format!("https://{addr}") accordingly; update the logic that sets http_endpoint
(same variable) and reuse the original endpoint variable for the suffix.

crates/grpc_client/src/trtllm_service.rs (1)

144-151: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Implement case-insensitive scheme matching.

The scheme checks use case-sensitive strip_prefix and won't handle mixed-case schemes like GRPC:// or GrPcS://. Past review analysis confirms that direct callers like worker.rs:952 pass URLs without normalization, and since grpc_base_url preserves original URL casing, mixed-case schemes will fail the conversion and cause connection errors.

🔧 Proposed fix

         // Convert gRPC schemes to tonic-compatible HTTP(S) endpoints.
-        let http_endpoint = if let Some(addr) = endpoint.strip_prefix("grpc://") {
+        let lower = endpoint.to_ascii_lowercase();
+        let http_endpoint = if let Some(_) = lower.strip_prefix("grpc://") {
+            let addr = &endpoint[7..];
             format!("http://{addr}")
-        } else if let Some(addr) = endpoint.strip_prefix("grpcs://") {
+        } else if let Some(_) = lower.strip_prefix("grpcs://") {
+            let addr = &endpoint[8..];
             format!("https://{addr}")
         } else {
             endpoint.to_string()
         };

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/grpc_client/src/trtllm_service.rs` around lines 144 - 151, The current
scheme matching uses case-sensitive strip_prefix on endpoint and will miss
mixed-case schemes; fix by computing a lowercase copy (e.g. let lower =
endpoint.to_ascii_lowercase()) and perform strip_prefix checks against lower
(e.g. lower.strip_prefix("grpc://") / lower.strip_prefix("grpcs://")), then map
the returned suffix back to the original endpoint string to build http_endpoint
so original host casing is preserved; update the logic that sets http_endpoint
(using endpoint and the strip_prefix results) to use these case-insensitive
checks instead of direct strip_prefix calls.

crates/grpc_client/src/vllm_engine.rs (1)

146-152: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Case-sensitive scheme check remains unfixed despite pipeline warning.

Pipeline failure warns: "Scheme check used to build http_endpoint in VllmEngineClient::connect is case-sensitive; mixed-case gRPC schemes can be missed and cause incorrect/failed HTTP(S) conversion."

The code continues to use case-sensitive strip_prefix, so URLs like GRPC://host:port or GRPCS://host:port from callers such as worker.rs:952 will fail to convert, resulting in connection failures.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/grpc_client/src/vllm_engine.rs` around lines 146 - 152, The
scheme-check in VllmEngineClient::connect uses case-sensitive
endpoint.strip_prefix which misses mixed-case schemes (e.g., "GRPC://"); update
the logic that builds http_endpoint to do a case-insensitive prefix check on
endpoint (e.g., compare a lowercase copy of endpoint) and then extract the
original address substring to construct "http://{addr}" or "https://{addr}" so
the host/port casing and rest remain intact; modify the code around
http_endpoint and the endpoint.strip_prefix usage to use the lowercase check but
derive addr from the original endpoint string.

crates/grpc_client/src/mlx_engine.rs (1)

125-131: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Case-sensitive scheme matching was flagged in previous reviews but not fixed.

The current implementation still uses case-sensitive strip_prefix, so schemes like GRPC://, GRPCS://, or GrPcS:// will not match and will cause connection failures. Direct callers (e.g., worker.rs:952 → GrpcClient::connect() → MlxEngineClient::connect()) pass URLs without normalization, making this a real production risk.

The fix was previously described in past review comments but has not been applied to the code.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/grpc_client/src/mlx_engine.rs` around lines 125 - 131, The scheme
matching is case-sensitive and should be made case-insensitive: in the
MlxEngineClient::connect() (called from GrpcClient::connect()), compute a
lowercased view of the endpoint (or use ASCII case-insensitive checks) to detect
prefixes like "grpc://" or "grpcs://", but when building the http/https URL use
the original endpoint substring (e.g., endpoint[prefix.len()..]) so you preserve
casing in the host/path; update the branch that sets http_endpoint to use this
case-insensitive prefix detection on the endpoint variable.

crates/grpc_client/src/trtllm_service.rs (1)

145-151: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Case-sensitive scheme matching flagged by pipeline and past reviews.

Pipeline failure explicitly warns: "Scheme matching in TrtllmServiceClient::connect is case-sensitive (e.g., via strip_prefix("grpc://") / strip_prefix("grpcs://")), so mixed-case gRPC schemes (e.g., GRPCS://) may not be converted correctly to HTTP(S) endpoints."

The current code still uses case-sensitive matching, which will cause connection failures for mixed-case schemes from direct callers like worker.rs:952.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/grpc_client/src/trtllm_service.rs` around lines 145 - 151, The scheme
matching is case-sensitive in TrtllmServiceClient::connect (the http_endpoint
logic uses endpoint.strip_prefix("grpc://") / "grpcs://"), so normalize
comparison by using a lowercased view for matching but slice the original
endpoint to preserve address casing: create endpoint_lower =
endpoint.to_ascii_lowercase() (or similar), use
endpoint_lower.starts_with("grpc://") / "grpcs://") to detect the scheme, then
compute the addr by slicing the original endpoint after the matched prefix
length and format the http/https URL into http_endpoint; replace the current
strip_prefix checks with this case-insensitive approach.

crates/grpc_client/src/sglang_scheduler.rs (1)

146-152: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Case-sensitive scheme matching remains unfixed.

Despite previous review comments identifying this issue, the code still uses case-sensitive strip_prefix("grpc://") and strip_prefix("grpcs://"). Mixed-case schemes like GRPC:// or GRPCS:// from direct callers (e.g., bindings/golang/src/policy.rs:364, worker.rs:952) will fail to convert and cause connection failures.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/grpc_client/src/sglang_scheduler.rs` around lines 146 - 152, The
scheme check on endpoint is case-sensitive and misses mixed-case schemes; change
the logic that sets http_endpoint (using the endpoint variable) to compare the
scheme case-insensitively: compute a lowercase copy like let lower =
endpoint.to_ascii_lowercase(), detect prefixes on lower (e.g.,
lower.strip_prefix("grpc://") / "grpcs://"), and when a prefix matches, slice
the original endpoint to preserve host casing (use the prefix length to take the
substring from endpoint) before formatting into "http://{addr}" or
"https://{addr}". Ensure this replaces the current strip_prefix branches that
use endpoint.strip_prefix directly.

model_gateway/src/workflow/steps/local/detect_connection.rs (1)

19-27: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Make explicit scheme detection case-insensitive to match util.rs.

The helper uses case-sensitive starts_with checks while util.rs parses schemes case-insensitively (url_scheme lowercases, strip_scheme uses eq_ignore_ascii_case). A URL like GRPC://host:30001 bypasses the explicit-scheme short-circuit and falls back to parallel probing. RFC 3986 defines schemes as case-insensitive.

🔧 Proposed fix

 fn explicit_connection_mode(url: &str) -> Option<ConnectionMode> {
-    if url.starts_with("grpc://") || url.starts_with("grpcs://") {
-        Some(ConnectionMode::Grpc)
-    } else if url.starts_with("http://") || url.starts_with("https://") {
-        Some(ConnectionMode::Http)
-    } else {
-        None
-    }
+    let scheme = url.split_once("://")?.0.to_ascii_lowercase();
+    match scheme.as_str() {
+        "grpc" | "grpcs" => Some(ConnectionMode::Grpc),
+        "http" | "https" => Some(ConnectionMode::Http),
+        _ => None,
+    }
 }

Also add a test for mixed-case input (e.g., GRPC://localhost:30001).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@model_gateway/src/workflow/steps/local/detect_connection.rs` around lines 19
- 27, The explicit_connection_mode function uses case-sensitive starts_with
checks so schemes like "GRPC://..." are missed; change it to detect the scheme
case-insensitively by comparing the scheme portion with eq_ignore_ascii_case (or
by lowercasing a small prefix) and return Some(ConnectionMode::Grpc) for "grpc"
/ "grpcs" and Some(ConnectionMode::Http) for "http" / "https"; update
explicit_connection_mode to check the scheme with eq_ignore_ascii_case (e.g.,
check the prefix before "://") and add a unit test that passes mixed-case input
such as "GRPC://localhost:30001" to verify the behavior.

✅ Autofix completed

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

❌ Autofix failed (check again to retry)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

model_gateway/src/workflow/steps/local/detect_connection.rs (1)

19-27: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make explicit scheme detection case-insensitive.

Line 20–23 currently use case-sensitive prefix checks. Mixed-case explicit schemes (for example, GRPC://...) skip the explicit branch and can fall back to HTTP-preferred probing, which breaks the explicit-scheme contract.

🔧 Proposed fix

 fn explicit_connection_mode(url: &str) -> Option<ConnectionMode> {
-    if url.starts_with("grpc://") || url.starts_with("grpcs://") {
-        Some(ConnectionMode::Grpc)
-    } else if url.starts_with("http://") || url.starts_with("https://") {
-        Some(ConnectionMode::Http)
-    } else {
-        None
-    }
+    let scheme = url.split_once("://")?.0.to_ascii_lowercase();
+    match scheme.as_str() {
+        "grpc" | "grpcs" => Some(ConnectionMode::Grpc),
+        "http" | "https" => Some(ConnectionMode::Http),
+        _ => None,
+    }
 }
@@
     fn explicit_connection_mode_honors_grpc_scheme() {
@@
         assert_eq!(
             explicit_connection_mode("grpcs://localhost:30001"),
             Some(ConnectionMode::Grpc)
         );
+        assert_eq!(
+            explicit_connection_mode("GrPc://localhost:30001"),
+            Some(ConnectionMode::Grpc)
+        );
     }

Also applies to: 126-158

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@model_gateway/src/workflow/steps/local/detect_connection.rs` around lines 19
- 27, The explicit_connection_mode function uses case-sensitive starts_with
checks so schemes like "GRPC://" are missed; change it to compare a lowercased
view of the URL (e.g., let lower = url.to_ascii_lowercase()) and run the prefix
checks against that (check "grpc://", "grpcs://", "http://", "https://"), and
apply the same case-insensitive prefix normalization to any other
explicit-scheme checks elsewhere in this file (the other explicit-scheme
detection block) so mixed-case schemes are handled consistently.